Tighten geotiff reader: partial-tile validation, ModelTransformation rotation guard#1491
Merged
Conversation
… rotation guard (#1486) Two accuracy fixes in the geotiff read path: 1. Validate decompressed tile/strip byte count in `_decode_strip_or_tile` before the reshape. A truncated deflate stream or a misbehaving compressor previously triggered an opaque "cannot reshape array of size N" message; now it raises ValueError with the expected size, actual size, and tile geometry. Edge tiles in valid TIFFs continue to decompress to the full tile_height x tile_width and pass the check unchanged. 2. Detect rotation, skew, and z-coupling in ModelTransformationTag (34264) and raise NotImplementedError. The previous code silently used only the diagonal terms, returning a corrupted GeoTransform for rotated rasters with no warning to the caller. Axis-aligned matrices keep working. Notes the audit also flagged a possible MinIsWhite + windowed-read issue, but the windowing path clamps to image bounds before allocating the output, so no padding ever reaches the inversion. Skipped. Adds 5 tests covering corrupt-tile detection, valid-edge-tile pass-through, axis-aligned ModelTransformation extraction, and the two error paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1486.
Summary
Two accuracy fixes in
xrspatial/geotiff/._decode_strip_or_tilenow validates the decompressed byte count against the expected size before reshape. A truncated deflate stream used to surface ascannot reshape array of size N into shape (h, w)with no hint of which tile or what was expected. New error names the tile geometry, expected size, actual size, and points at corruption._extract_transformnow checks ModelTransformationTag (34264) for non-zero rotation (M[1], M[4]) and z-coupling (M[2], M[6]) and raises NotImplementedError. The old code used only the diagonal terms and silently produced a corrupted GeoTransform on rotated rasters.The audit also flagged a possible MinIsWhite + windowed-read issue. On close reading the window is clamped to image bounds before allocating output, so padding never reaches the inversion. Skipped with a note in the issue.
Test plan
test_truncated_tile_raises_clear_error- corrupt deflate stream raises ValueError with the size mismatchtest_valid_edge_tile_still_works- 9x9 image with 4x4 tiles round-trips cleanly (edge tiles are partial)test_axis_aligned_extracts_correctly- ModelTransformationTag without rotation extracts origin/pixel sizestest_rotation_raises- non-zero M[1]/M[4] raises NotImplementedErrortest_z_coupling_raises- non-zero M[2]/M[6] raises NotImplementedErrorRecursionErrorfailures unrelated to this PR